Skip to content

Say the count of a coalesced event is in its tag, not its value - #43

Merged
dmccoystephenson merged 1 commit into
mainfrom
feature/docs-coalesced-count-formula
Nov 7, 2026
Merged

Say the count of a coalesced event is in its tag, not its value#43
dmccoystephenson merged 1 commit into
mainfrom
feature/docs-coalesced-count-formula

Conversation

@dmccoystephenson

@dmccoystephenson dmccoystephenson commented Nov 7, 2026

Copy link
Copy Markdown
Member

Summary

Stage A (documentation accuracy sweep) cycle. Every README section and the Javadoc were checked against the code. No production code is changed: the TraceClient.java diff is comments only.

Wrong claim corrected

  • The formula for how many times an event happened said to sum value for events tagged coalesced. That only holds for an event reported without a value. An event reported with a value keeps it (flush sends window.value when it is not null), so value is not the count and only the coalesced tag holds it. coalesce_keysOnNameValueAndTagsButNotTagOrder already pins this: value 2.5 with coalesced 2. The README also called command valueless while its own example reports command with 1.0, so a query following the old text would undercount that example. The README and the report Javadoc now give the tag as the count, and COALESCE(value, 1) is kept as the shortcut for valueless events.

Claims made more exact

  • A count that finds the queue full is lost (enqueue returns without queueing it, and flush has already reset or removed the window). The README said close() sends every count still open. This was not pinned by any test, so close_aCountThatFindsTheQueueFullIsDropped is added.
  • A report made after close() is dropped. Already pinned by report_afterCloseIsDroppedWithoutThrowing.
  • The install tag is left off an event that already holds 32 tags with version. The wire format section said install was present whenever the client has an ID. Already pinned by withInstall_addsTheIdOnlyWhenThereIsOneAndRoomAndNoneAlready.
  • A comment at column 0 does not end a tags: block. The README and the parseServerWideConfig Javadoc said the block ends at the next line that is not indented. Already pinned by serverWideTags_skipBlankAndCommentLinesInsideTheBlock.
  • The server-wide switch accepts off, 0 and no in any case, as well as false. Stated in the class and Builder.serverWideConfig Javadoc. Already pinned by serverWideConfig_acceptsEverySpellingOfOff.
  • installIdFromFile writes nothing for a null file or a blank path. Already pinned by installIdFromFile_neverThrowsForABadPath.

Merge gate

The README "Opting out" and "Keys" sections, pom.xml, the workflows, the version strings and every wire-format or opt-out constant are untouched, so no do-not-auto-merge path matches. The README row for the spelling fix above is in the protected "Opting out" section, so it was left out and filed as #42.

Triage notes (skipped issues)

No issue is closed by this PR. #13, #15, #27, #31, #39 and #40 each ask for a maintainer decision on the wire format or the opt-out contract, and each lists options, so none was picked up for autonomous implementation. #42 is in the same position. The body of #31 quotes the old formula. Its substance still holds, because a count sent without the tag is lost under either formula.

Test plan

  • mvn -B verify run locally on Java 21: Tests run: 107, Failures: 0, Errors: 0, Skipped: 0 (106 before this change)
  • The new test was confirmed able to fail: with the bound in enqueue temporarily raised by one, it went red with the count in the delivered bodies, and the file was restored afterwards
  • CI matrix green on Java 8, 17 and 21

CI does not cover anything host-specific here, and nothing in this PR depends on a real server.

No tracking issue: these gaps were found during triage.

This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

🤖 Generated with Claude Code

drafted by Claude on behalf of Daniel Stephenson

@dmccoystephenson @claude
Say the count of a coalesced event is in its tag, not its value
3433c39 
The documented formula summed `value` for events tagged `coalesced`, which
is only right for an event reported without a value: one reported with a
value keeps it, and only the tag holds the count. The README's own example
reports `command` with 1.0, and its text called `command` valueless.

Also say what the code already does: a count that finds the queue full is
lost (pinned by a new test), a report after close() is dropped, install is
left off at 32 tags, a comment at column 0 does not end a tags: block, the
server-wide switch takes the same spellings as isOff, and installIdFromFile
writes nothing for a null or blank path.

No production code changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dmccoystephenson

dmccoystephenson commented Nov 7, 2026

Copy link
Copy Markdown
Member Author

Self-review rubric (posted as a comment; this is not an independent review):

  • Scope: PASS. Three files changed. Every hunk is a documentation correction from the Stage A sweep or the one test pinning a claim that had no test.
  • Tests-new: PASS. No public method is added. close_aCountThatFindsTheQueueFullIsDropped pins the one new claim that was not yet covered.
  • Tests-fix: not applicable, as no bug fix is made. The new test was still shown able to fail: raising the enqueue bound by one turned it red, and the file was restored.
  • Sibling structure: PASS. No new file. The test follows the Arrange/Act/Assert layout and the local slow server pattern of queue_isBoundedAndDropsRatherThanGrows.
  • Sibling renames: PASS. Nothing is renamed.
  • Docs: PASS with a tracked gap. Every Phase 7 row was checked. The class Javadoc now lists the server-wide spellings while the protected README "Opting out" row does not, which is filed as The Opting out table lists only enabled: false for the server-wide switch #42.
  • Issue resolution: not applicable. The PR carries no closing reference.
  • CI: PASS. build (8), build (17) and build (21) are green on the PR head.

Repo-specific:

  • No dependencies: PASS. pom.xml is untouched, and all imports in TraceClient.java are java.*.
  • Single file: PASS. git ls-files src/main lists only TraceClient.java.
  • Java 8: PASS. build (8) is green, and the test uses no post-8 API.
  • Never throws, non-blocking and bounded: PASS. The TraceClient.java diff is comments only, confirmed by filtering the diff for lines outside Javadoc.
  • Environment seam: PASS. The environment is read only through the environment field, whose initializer is System::getenv, and this PR does not touch it.
  • Version agreement and opt-out contract: PASS. No version string, REASON_*, ENV_* or SERVER_WIDE_CONFIG_CONTENT is touched, and the README "Opting out" table is unchanged.

Findings:

  • src/main/java/software/stephenson/trace/TraceClient.java:545 — the rewrapped parseServerWideConfig Javadoc leaves one short line ("first entry. Values may be bare,"). Cosmetic. It is left as it stands so that the unchanged lines below it are not reflowed.
  • src/test/java/software/stephenson/trace/TraceClientTest.java:2256 — the final assertion checks that no delivered body carries the tag, so it could pass vacuously if close() stopped before the queue drained. The mutation run shows the count does arrive within the 5 s bound when it is queued. An exact body count was avoided because queue_isBoundedAndDropsRatherThanGrows records a stray failed send on Java 8 CI.

Do-not-auto-merge check: no listed path matches. The README "Opting out" and "Keys" sections, .github/workflows/* and pom.xml are untouched, no single file loses more than 50 lines, and no wire-format or opt-out code changes.

This comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

drafted by Claude on behalf of Daniel Stephenson

@dmccoystephenson
dmccoystephenson merged commit 6e03891 into main Nov 7, 2026
3 checks passed
@dmccoystephenson
dmccoystephenson deleted the feature/docs-coalesced-count-formula branch November 7, 2026 04:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@dmccoystephenson